🛡️ Sentinel: [HIGH] Harden Windows command execution against injection#799
🛡️ Sentinel: [HIGH] Harden Windows command execution against injection#799
Conversation
- Enhance `cmd_escape` in `src/utils/mod.rs` to escape `%` characters as `%""` to prevent environment variable expansion in `cmd.exe`. - Update `validate_command_args` in `src/modules/mod.rs` to treat `%` and `^` as dangerous patterns, preventing their use in raw command strings where they could facilitate injection or information disclosure. - Add unit tests for both changes to verify security hardening. This change mitigates potential command injection and unintended variable expansion vulnerabilities on Windows systems. Co-authored-by: dolagoartur <146357947+dolagoartur@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a70204ab8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ("%", "variable expansion %"), | ||
| ("^", "cmd.exe escape ^"), |
There was a problem hiding this comment.
%/^ rejection to Windows command paths
Adding % and ^ to the global dangerous_patterns list makes validate_command_args fail for these characters on every platform, but this validator is called unconditionally by modules like CommandModule::validate_params (src/modules/command.rs) and ServiceConfig::from_params (src/modules/service.rs) before any Windows shell selection. This creates a cross-platform regression where legitimate non-Windows arguments (for example format strings containing % or regex anchors with ^) are now rejected even when no cmd.exe expansion is involved.
Useful? React with 👍 / 👎.
| } else if c == '%' { | ||
| // Insert empty string ("") after % to prevent variable expansion in cmd.exe | ||
| // e.g. %VAR% becomes %""VAR%"" | ||
| escaped.push_str("%\"\""); | ||
| } else { |
There was a problem hiding this comment.
cmd_escape
Rewriting % to %"" is not a stable literal-escaping strategy for cmd.exe and can change the payload instead of preserving it as text (notably %NAME%-style substrings can be interpreted as a variable token with a different name). Since cmd_escape is used when building Windows command strings from argv (src/modules/command.rs), arguments containing literal percent markers can be corrupted at execution time rather than passed through safely.
Useful? React with 👍 / 👎.
1 participant
🛡️ Sentinel: [HIGH] Harden Windows command execution against injection
🚨 Severity: HIGH
💡 Vulnerability: Windows command execution via
cmd.exeallows environment variable expansion (%VAR%) and special character escaping (^) even inside double quotes (for%) or in raw strings. This could allow attackers to inject commands or disclose environment variables if they can control part of the command string.🎯 Impact: Potential Remote Code Execution (RCE) or Information Disclosure on Windows hosts if user input is used to construct commands.
🔧 Fix:
1. Modified
cmd_escapeto neutralize%by injecting empty strings (%""), which breaks variable tokens incmd.exewithout altering the literal string value.2. Updated
validate_command_argsto reject%and^in raw command strings, forcing a "fail-closed" behavior for these dangerous characters.✅ Verification: Added unit tests
test_cmd_escape(verifying%escaping) andtest_validate_command_args_rejects_windows_metachars(verifying rejection of%and^). Confirmed that all module tests pass.PR created automatically by Jules for task 9490688386948733044 started by @dolagoartur